Separate the various parts of the error report with newlines #11659
Conversation
Previously the error report would have all sections glued together: - The assertion representation - The error explanation - The full diff This makes it hard to see at a glance where which one starts and ends. One of the representation (dataclasses, tuples, attrs) does display a newlines at the start already. Let's add a newlines before the error explanation and before the full diff, so we get an easier to read report. This has one disadvantage: we get one line less in the least verbose mode, where the output gets truncated.
There was a problem hiding this comment.
I like it! Just one question.
The non-verbose mode is now slightly worse (outside CI), with the output being truncated. Do we want to cater for that case and not add the newline in that case?
It seems OK to me, i.e. no need to special case.
This improvement is part of an issue that already has a changelog entry. Should I merge both or how should the file be named? (I currently added a .2 to it)
You can add it to the previous changelog entry, or you can add a new one. I plan to hand-edit the pytest 8 release notes anyway to make them more coherent so we can merge them then.
| r" comparison failed. Mismatched elements: 20 / 20:", | ||
| rf" Max absolute difference: {SOME_FLOAT}", | ||
| rf" Max relative difference: {SOME_FLOAT}", | ||
| r" Index \| Obtained\s+\| Expected", | ||
| rf" \(0,\)\s+\| {SOME_FLOAT} \| {SOME_FLOAT} ± {SOME_FLOAT}", | ||
| rf" \(1,\)\s+\| {SOME_FLOAT} \| {SOME_FLOAT} ± {SOME_FLOAT}", | ||
| rf" \(2,\)\s+\| {SOME_FLOAT} \| {SOME_FLOAT} ± {SOME_FLOAT}...", |
There was a problem hiding this comment.
I assume the ... here is added by pytest to indicate truncation. If so, shouldn't it be now on the previous line?
There was a problem hiding this comment.
Woops good catch. I've updated the test to add it back
There was a problem hiding this comment.
Just to make sure, these are literal dots, and the assert is a regex, right? If so, it would be nice to escape the .s to make it clear and not overmatch: \.\.\.. And maybe also add anchors ^ and $.
There was a problem hiding this comment.
Good catch, I've made the whole test regexes more restrictive. Mind having another look at that part?
Thanks I added it to the previous one. |
There was a problem hiding this comment.
Thanks @BenjaminSchubert, LGTM.
(Note: when a PR is approved you're free to merge, but let's keep this open for a couple of days to let others review if they'd like)
| r" comparison failed. Mismatched elements: 20 / 20:", | ||
| rf" Max absolute difference: {SOME_FLOAT}", | ||
| rf" Max relative difference: {SOME_FLOAT}", | ||
| r" Index \| Obtained\s+\| Expected", | ||
| rf" \(0,\)\s+\| {SOME_FLOAT} \| {SOME_FLOAT} ± {SOME_FLOAT}", | ||
| rf" \(1,\)\s+\| {SOME_FLOAT} \| {SOME_FLOAT} ± {SOME_FLOAT}", | ||
| rf" \(2,\)\s+\| {SOME_FLOAT} \| {SOME_FLOAT} ± {SOME_FLOAT}...", |
There was a problem hiding this comment.
Just to make sure, these are literal dots, and the assert is a regex, right? If so, it would be nice to escape the .s to make it clear and not overmatch: \.\.\.. And maybe also add anchors ^ and $.
|
Perfect thanks :) I'll aim to merge this on wednesday unless I get more review by then |
Description
This is part of #11520.
Previously the error report would have all sections glued together:
This makes it hard to see at a glance where which one starts and ends.
One of the representation (dataclasses, tuples, attrs) does display a newlines at the start already.
Let's add a newlines before the error explanation and before the full diff, so we get an easier to read report.
This has one disadvantage: we get one line less in the least verbose mode, where the output gets truncated (outside CI).
Examples
Without pygments installed
Before
After
With pygments installed
Before
After
Questions
.2to it)